Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: New Global Controls in the Layer Picker #352

Merged
merged 20 commits into from
Jan 22, 2025
Merged

feat: New Global Controls in the Layer Picker #352

merged 20 commits into from
Jan 22, 2025

Conversation

popkinj
Copy link
Collaborator

@popkinj popkinj commented Jan 17, 2025 •
edited by github-actions bot
Loading

Features

  • Close all layer groups button
  • Open all layer groups button
  • Un-select all layers is now at the top matching the other global controls
  • Global layer description and link has been moved to the bottom
  • Small fix to the baselayer switcher to allow for the e2e tests to pass

Screenshot from 2025-01-17 09-50-08

Screenshot from 2025-01-17 09-50-20

Screenshot from 2025-01-17 09-50-32

Thanks for the PR!

Deployments, as required, will be available below:

Please create PRs in draft mode. Mark as ready to enable:

After merge, new images are deployed in:

@popkinj popkinj requested review from dawnbcgov and kdbelair January 17, 2025 19:58
This was linked to issues Jan 17, 2025
Data Layers > Add the ability to expand all /collapse layers at the top of the data layers section #251
Closed
Data Layers > Move the "reset layers" button position to the top #252
Closed
@popkinj popkinj marked this pull request as ready for review January 17, 2025 20:08
@kdbelair
Copy link

kdbelair commented Jan 17, 2025 •
edited
Loading

Thanks @popkinj Are we able to show how many layers are selected and have the button colour change when selected as shown in Figma? I would also like to have the button changed from "Data Layers" to "Layer's as shown in Figma. The new buttons are working well!
Layers

@popkinj
Copy link
Collaborator Author

popkinj commented Jan 17, 2025 •
edited
Loading

The styling is not finished. But the functionality is there. I'm planning to overhaul all the button styles at the same time.

Screenshot from 2025-01-17 13-59-52

Screenshot from 2025-01-17 13-46-29

@popkinj popkinj linked an issue Jan 18, 2025 that may be closed by this pull request
Fix outstanding issues from #343 Basemap switcher #351
Closed
2 tasks
@popkinj
Copy link
Collaborator Author

popkinj commented Jan 18, 2025

Fixing basemap switcher issues... with styling.
Screenshot from 2025-01-18 14-03-51
Screenshot from 2025-01-18 14-05-48

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'nr-omrr-transparency-frontend'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mishraomp
mishraomp reviewed Jan 20, 2025
View reviewed changes
Copy link
Contributor

@mishraomp mishraomp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@DerekRoberts DerekRoberts enabled auto-merge (squash) January 20, 2025 18:17
@DerekRoberts DerekRoberts disabled auto-merge January 20, 2025 18:17
@dawnbcgov
Copy link
Contributor

dawnbcgov commented Jan 20, 2025 •
edited
Loading

It seems like the buttons state on mobile and tablet isn't clearing, screenshots below:

image

image

Actually, sorry, I reviewed these:

  • they have the wrong design token, it's showing as --surface-color-primary-active-border, but it should be --surface-color-primary-default, that's why to my brain they look like they are active all the time :)
  • also it looks like there is a hard-coded border radius that is different than the other buttons (--layout-border-radius-medium which is 4px is used elsewhere currently I think the token you should use is --layout-border-radius-large )

@kdbelair
Copy link

The find me icon on mobile and tablet does not clear after moving away from location
findme

@dawnbcgov
Copy link
Contributor

dawnbcgov commented Jan 21, 2025 •
edited
Loading

Needs:

  • added documentation about layer limits (not behind authentication)

mishraomp
mishraomp approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@mishraomp mishraomp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@popkinj popkinj merged commit b352eb0 into main Jan 22, 2025
20 checks passed
@popkinj popkinj deleted the map/layers-251 branch January 22, 2025 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mishraomp mishraomp mishraomp approved these changes

@dawnbcgov dawnbcgov Awaiting requested review from dawnbcgov

@kdbelair kdbelair Awaiting requested review from kdbelair

@DerekRoberts DerekRoberts Awaiting requested review from DerekRoberts DerekRoberts is a code owner

@AmanBhathal AmanBhathal Awaiting requested review from AmanBhathal AmanBhathal is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@popkinj @kdbelair @dawnbcgov @mishraomp